[optimizer] consume KVCM event streams and report online MRC metrics - #296

Preserve cache-query semantics before replaying events

When KVCM serves QT_BATCH_GET or QT_REVERSE_ROLL_SW_MATCH, this path replays the event unconditionally as a full-attention prefix query. CacheManager records query_type in CacheGetEvent, but OptimizerEventPublisher::Convert does not serialize it, and auto-created optimizer groups enable prefix hashing, so independent batch keys are rolling-hashed and interpreted as one prefix chain. For workloads using these query types, hit rates and MRC values are therefore unrelated to the actual accesses; either filter the stream to compatible prefix-match events or carry the query type through the protocol and handle each form correctly.

Useful? React with 👍 / 👎.

Handle instances with no location-spec groups

KVCM permits legacy instances to omit location_spec_groups—the field defaults to an empty vector—but this check only classifies instances with more than one group as unsupported. A zero-group instance therefore reaches OnlineOptimizerManager::RegisterInstance, which rejects it because implicit full-only registration requires exactly one group; ApplyKvcmConfiguration then fails the entire snapshot, SyncConfiguration never calls UpdateWorker, and no events are consumed even for otherwise valid instances. Treat zero-group instances as unsupported or synthesize a full group from their location_spec_infos.

Useful? React with 👍 / 👎.

Avoid accumulating MRC data when interval reporting is disabled

When metrics_report_interval_ms is zero or negative, OnlineOptimizerServer::Start intentionally creates no reporting thread, but every theoretical query still records into mrc_window. Because TakeMrcMetrics is then never called, its sparse map is never reset and retains every historical reuse-distance boundary, including entries for blocks later removed by the 24-hour TTL; a long-running optimizer with interval reporting disabled can therefore accumulate substantial unbounded historical metric state. Skip MRC recording in this mode or provide another bounded/reset path.

Useful? React with 👍 / 👎.

Preserve the existing registration response field name

Renaming protobuf field 3 from size_full_only to size_full is wire-compatible for binary gRPC, but it breaks the public source and HTTP contracts: regenerated clients no longer have the size_full_only accessor, and this repository's JSON serializer uses preserve_proto_field_names, so /api/optimizer/registerInstance now returns size_full instead of size_full_only. Existing HTTP consumers that read the old field silently lose the block-size result; retain the protobuf name and limit the terminology rename to internal C++ state, or provide an explicit compatibility path.

Useful? React with 👍 / 👎.

Exclude intentionally unsupported events from error metrics

For an instance deliberately placed in unsupported_instance_ids_ because it has multiple location-spec groups, ExecuteTraceQuery returns EC_INSTANCE_NOT_EXIST, and this block reports it as a service error before the later unsupported-instance check suppresses the log and refresh. Every access to such a known unsupported instance therefore increments service.error_counter/error QPS, potentially dominating alerts in mixed-model deployments even though the subscriber is behaving as configured. Check the unsupported set before reporting the error, or classify these drops separately.

Useful? React with 👍 / 👎.

Bracket IPv6 leader addresses before appending the port

When GetClusterInfo returns an IPv6 literal, this produces an ambiguous target such as 2001:db8::1:6381 rather than the gRPC address [2001:db8::1]:6381. Consequently, an optimizer that successfully reaches an IPv6 seed cannot connect to the discovered leader for GetConfiguration or SubscribeEvents; format IPv6 hosts with brackets before constructing the endpoint.

Useful? React with 👍 / 👎.

Account for location specs before replaying block keys

When a supported prefix query supplies per-key location_spec_names, this call discards them and replays only block_keys. KVCM treats the spec as part of a block's logical identity and uses each per-position name to select cache candidates, so accesses such as (key, F0) followed by (key, L1) can have different serving outcomes while the optimizer treats the latter as a hit on the former; the configured full-group byte charge is also inaccurate for such partial-spec accesses. This corrupts hit-rate and MRC results even without the unsupported query types noted elsewhere, so nonempty spec-qualified events should either be modeled with spec-aware identities/charges or excluded from replay.

Useful? React with 👍 / 👎.

Clear KMonitor state when MRC registration fails

If KMonitor accepts the existing metrics but rejects the newly added mrc metric, this returns failure while leaving kmon_ctx_, its kmonitor pointer, and a null mrc_metrics intact. OnlineOptimizerServer::Init explicitly continues after InitKmonitor() returns false, and once a full-attention instance exists the next interval report passes its context checks and dereferences mrc_metrics in ReportInterval, crashing the optimizer instead of running with KMonitor disabled. Reset the KMonitor context on this failure path, as is already done for earlier initialization failures.

Useful? React with 👍 / 👎.

Skip zero-quota groups instead of aborting synchronization

When a KVCM snapshot contains an instance group whose quota capacity is zero or negative, this rejects the entire snapshot. KVCM itself permits these capacities—InstanceGroupQuota::ValidateRequiredFields imposes no positivity constraint, and the serving/reclaimer paths interpret nonpositive capacity as a group with no available quota—while GetConfiguration emits every registered group. Thus one intentionally disabled group prevents SyncConfiguration from calling UpdateWorker, so events are not consumed for any otherwise valid positive-capacity group; classify the nonpositive group and its instances as unsupported rather than failing the complete synchronization.

Useful? React with 👍 / 👎.

Try alternate seeds when the discovered leader is unusable

If the first reachable seed returns a stale or temporarily unreachable leader endpoint, DiscoverLeader stops at that seed and this failure returns from the refresh without consulting the remaining healthy seeds. Because every refresh repeats the same ordering, a stale first seed can prevent configuration synchronization and event consumption indefinitely even when another seed reports a usable current leader; continue discovery with the remaining seeds when SyncConfiguration fails.

Useful? React with 👍 / 👎.

Avoid access-logging every streamed cache event

When event subscription is enabled, every cache access passes through this OptimizerCallGuard, whose destructor unconditionally emits KVCM_ACCESS_LOG. Unlike an optimizer RPC, this path can run at the full KVCM cache-query rate, so it generates an additional access-log record per streamed event and can make logging I/O or disk volume dominate the single stream-processing thread, causing its bounded upstream queue to drop events. Keep the metrics/timing behavior without using the RPC guard's access-log side effect.

Useful? React with 👍 / 👎.

Keep producer-time TTL state from being advanced by wall time

When timestamped events are delayed longer than an instance's TTL, replaying the first event here creates state at the producer timestamp, but the periodic ListInstances call advances the same LiteHit state to the optimizer's current wall clock. If reporting occurs before the next queued producer-time event, it expires that state, so an event at producer time t+5 misses data inserted at t even when the TTL is 10 seconds. This defeats the producer-timestamp semantics added for queueing delays; TTL maintenance and reporting need to use a consistent event-time watermark.

Useful? React with 👍 / 👎.

Track accepted sequences across batches

When a producer resends a batch after losing the success response, this validation compares sequences only within the current request because previous_sequence is reset on every call and no progress is stored for producer_id. The repeated observations are therefore enqueued again, double-counting accesses and altering LRU/MRC results despite the sequence and last_accepted_sequence contract; retain the last accepted sequence per producer and reject or skip already accepted observations before enqueueing.

Useful? React with 👍 / 👎.

Reject negative observation timestamps before enqueueing

When an observation supplies a negative timestamp_ns, this validation accepts it and the response counts it as successfully enqueued, but OnlineOptimizerManager::TraceQuery rejects every negative timestamp with EC_BADARGS. The subscriber therefore drops the observation asynchronously after the producer has been told it was accepted; validate the timestamp here so the whole invalid batch is rejected before SendBatch.

Useful? React with 👍 / 👎.

Return the follower status that triggers leader discovery

When report_trace_batch reaches a stable follower after a leader change, this branch returns SERVICE_NOT_READY, while KvCacheManagerClient._make_api_request only rediscovers the leader and retries on SERVER_NOT_LEADER. The public client therefore fails this observation call even when a healthy leader is available; return SERVER_NOT_LEADER for the follower case and reserve SERVICE_NOT_READY for recovery or queue availability failures.

Useful? React with 👍 / 👎.